feat: configurable sensor debounce with stop-then-verify#37
Conversation
Add a "Sensor Debounce Seconds" number entity (disabled by default, 0-30s, default 0) that lets users configure how many consecutive seconds a sensor must read wet/dry before the pump reacts. Default 0 preserves original behavior. Also fix typo in output sensor ID (fulid → fluid) and add debounce to auto-refill trigger.
WalkthroughReplaces counter-based debounce with per-sensor timestamp globals, adds a configurable ChangesPump debounce & safety
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Integrations/ESPHome/Core.yaml (1)
281-296:⚠️ Potential issue | 🟠 MajorQueued
on_releasetimers can fire after a wet bounce.Line 283 queues a delayed refill check on every release, but a later wet transition does not cancel the earlier timer. In ESPHome, delayed actions in
on_releaseautomations are not automatically canceled when the sensor transitions back toon(on_press). A pattern likeOFF → ON → OFFcan still satisfy the first timer after less than the configured continuousOFFtime, breaking the consecutive-seconds debounce contract.⏱️ Suggested pattern
- platform: gpio name: Fluid Input id: fluid_input_sensor @@ on_release: then: - - delay: !lambda 'return (int)id(sensor_debounce_seconds).state * 1000;' - - if: - condition: - and: - - binary_sensor.is_off: fluid_input_sensor - - switch.is_on: auto_refill - - switch.is_on: tank_refill_mode - - switch.is_off: pump_control - then: - - logger.log: "Auto refill triggered - tank level low" - - text_sensor.template.publish: - id: last_pump_action - state: "Auto Refill Triggered" - - script.execute: pumpUntilFull + - script.execute: auto_refill_debounce + on_press: + then: + - script.stop: auto_refill_debounceAdd a restartable script elsewhere so each new edge replaces the old timer:
script: - id: auto_refill_debounce mode: restart then: - delay: !lambda 'return (int)id(sensor_debounce_seconds).state * 1000;' - if: condition: and: - binary_sensor.is_off: fluid_input_sensor - switch.is_on: auto_refill - switch.is_on: tank_refill_mode - switch.is_off: pump_control then: - logger.log: "Auto refill triggered - tank level low" - text_sensor.template.publish: id: last_pump_action state: "Auto Refill Triggered" - script.execute: pumpUntilFull🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Integrations/ESPHome/Core.yaml` around lines 281 - 296, The on_release handler currently delays and then runs the refill check without cancellation, allowing queued timers to fire after a wet bounce; refactor by creating a restartable script (e.g., id: auto_refill_debounce) that performs the delay using id(sensor_debounce_seconds) and then checks the conditions (binary_sensor: fluid_input_sensor, switches: auto_refill, tank_refill_mode, pump_control) and executes the same actions (logger log, text_sensor.publish to last_pump_action, script.execute: pumpUntilFull); replace the on_release delayed block with a call to this script so each edge restarts/cancels the previous timer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Integrations/ESPHome/Core.yaml`:
- Around line 467-498: The output wet-count condition incorrectly requires
stop_pump_when_full to be OFF during tank_refill_mode; change the condition that
increments id(output_wet_count) for the refill path so it checks
id(tank_refill_mode).state && id(stop_pump_when_full).state &&
id(fluid_input_sensor).state (i.e., require the stop_pump_when_full switch to be
ON in refill mode) instead of the current negated check; adjust only the
condition in the lambda that increments id(output_wet_count) (referencing
stop_pump_when_full, tank_refill_mode, fluid_input_sensor, fluid_output_sensor,
and output_wet_count) so the input-sensor full-stop path is armed when refill
mode is active.
---
Outside diff comments:
In `@Integrations/ESPHome/Core.yaml`:
- Around line 281-296: The on_release handler currently delays and then runs the
refill check without cancellation, allowing queued timers to fire after a wet
bounce; refactor by creating a restartable script (e.g., id:
auto_refill_debounce) that performs the delay using id(sensor_debounce_seconds)
and then checks the conditions (binary_sensor: fluid_input_sensor, switches:
auto_refill, tank_refill_mode, pump_control) and executes the same actions
(logger log, text_sensor.publish to last_pump_action, script.execute:
pumpUntilFull); replace the on_release delayed block with a call to this script
so each edge restarts/cancels the previous timer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c28e2629-d1f3-404b-8100-f18df1ec2812
📒 Files selected for processing (1)
Integrations/ESPHome/Core.yaml
Add a "Sensor Debounce Seconds" number entity (disabled by default, 0-30s, default 0) for water level sensors. Uses stop-then-verify approach: pump stops immediately when a sensor triggers, waits the configured seconds, then re-checks. If the reading was a false alarm (e.g. wave bounce), the pump restarts automatically. Safety timeout is preserved across debounce restarts. Also fix typo in output sensor ID (fulid -> fluid) and add debounce to auto-refill trigger.
bharvey88
changed the title
bharvey88
changed the title
|
Rewrote from stop-then-verify to a timestamp-based filter. Why: the original mechanism cycled the pump on/off during sloshing at the fill threshold, which is exactly the case this feature is supposed to handle. It also silently broke New approach: CodeRabbit comments:
Untested on hardware; CI builds the firmware. Flash test recommended before merge. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Integrations/ESPHome/Core.yaml (1)
521-532:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winInverted/refill mode stop condition will never trigger.
The condition at line 526 requires
stop_pump_when_fullto be OFF, but whentank_refill_modeis enabled (line 164-168), it automatically turnsstop_pump_when_fullON. These conditions are mutually exclusive, so the "destination full" stop in refill mode can never fire.The condition should check that
stop_pump_when_fullis ON (consistent with how it gates the normal-mode stop at line 503):🐛 Proposed fix
# Destination full in inverted/refill mode: input sensor wet for >= debounce_seconds - if: condition: - and: - switch.is_on: tank_refill_mode - - switch.is_off: stop_pump_when_full + - switch.is_on: stop_pump_when_full - lambda: |- return id(input_wet_since) != 0 && (millis() - id(input_wet_since)) / 1000 >= (uint32_t)id(sensor_debounce_seconds).state;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Integrations/ESPHome/Core.yaml` around lines 521 - 532, The inverted/refill-mode stop condition is impossible because it checks switch.is_off: stop_pump_when_full while tank_refill_mode sets stop_pump_when_full on; change the condition to require switch.is_on: stop_pump_when_full so the block that checks id(input_wet_since) and (millis() - id(input_wet_since)) / 1000 >= (uint32_t)id(sensor_debounce_seconds).state can execute and then run the logger and switch.turn_off: pump_control; update the condition containing tank_refill_mode and stop_pump_when_full accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@Integrations/ESPHome/Core.yaml`:
- Around line 521-532: The inverted/refill-mode stop condition is impossible
because it checks switch.is_off: stop_pump_when_full while tank_refill_mode sets
stop_pump_when_full on; change the condition to require switch.is_on:
stop_pump_when_full so the block that checks id(input_wet_since) and (millis() -
id(input_wet_since)) / 1000 >= (uint32_t)id(sensor_debounce_seconds).state can
execute and then run the logger and switch.turn_off: pump_control; update the
condition containing tank_refill_mode and stop_pump_when_full accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81f46284-dc68-4ba8-9ecb-d660c7972d02
📒 Files selected for processing (1)
Integrations/ESPHome/Core.yaml
|
Customer-reported regression: in refill mode, the pump did not stop when the output sensor went wet, and a tank overflowed. Cause: the previous timestamp-refactor commit added Fix: removed the gate. The three stop branches are already mutually exclusive via This is independent of the sensor-bouncing concern the customer also raised. If filtering still feels inadequate after this fix, the next thing to investigate is the 1Hz poll rate inside Pushed in |
|
Actionable comments posted: 0 |
bharvey88
force-pushed
the
feat/configurable-debounce
branch
from
889ebda to
52a6748
Compare
|
Reverted via force-push: branch is back to 52a6748 (the stop-then-verify version, pre-rewrite). My timestamp-refactor commits (90b062c, 889ebda) failed in customer testing. Going back to known state so the customer can reflash and use the device. We'll re-attempt the debounce work later with better test coverage. |
bharvey88
changed the title
2 participants
Version: 25.04.12.1
What does this implement/fix?
Adds a configurable "Sensor Debounce Seconds" number entity that prevents false sensor readings (caused by CPAP air intake waves) from causing erratic pump behavior.
How it works (stop-then-verify):
This is safer than delaying the stop — the pump is never running while a stop condition is active.
Configuration:
Safety:
max_safe_run_time) is preserved across debounce restarts — a bouncy sensor cannot keep the pump running indefinitelydebounce_restartglobal flagAlso included:
fulid_output_sensor→fluid_output_sensor)on_release) respects debounce delay with check-wait-confirm patternTypes of changes
Checklist / Checklijst:
If user-visible functionality or configuration variables are added/modified: